SLVS-2465 Add analysis engine to connect with Roslyn APIs #6368

georgii-borovinskikh-sonarsource · 2025-08-08T10:57:27Z

Part of

gabriela-trutan-sonarsource

Please rename all classes to not contain Sonar. The product name makes the class names longer and it does not bring any clarity on their responsibility.

Consider grouping the files inside the Analysis folder into different sub-folders. For now it's very difficult to find the classes.

I also have some additional feedback.

src/RoslynAnalyzerServer/Analysis/Wrappers/SonarRoslynProjectWrapper.cs

src/RoslynAnalyzerServer/Analysis/DiagnosticDuplicatesComparer.cs

src/RoslynAnalyzerServer.UnitTests/Analysis/DiagnosticDuplicatesComparerTests.cs

src/RoslynAnalyzerServer.UnitTests/Analysis/SequentialSonarRoslynAnalysisEngineTests.cs

src/RoslynAnalyzerServer/Analysis/SonarRoslynProjectAnalysisSet.cs

gabriela-trutan-sonarsource · 2025-08-13T14:16:02Z

src/RoslynAnalyzerServer/Analysis/SonarRoslynSolutionAnalysisCommandProvider.cs

+
+        foreach (var filePath in filePaths)
+        {
+            if (!project.ContainsDocument(filePath, out var analysisFilePath))


For each project we are iterating multiple times through the files (inside SonarRoslynProjectWrapper.ContainsDocument), which can have a real hit on performance. Instead we should cache the list of file names and the filtering applied to them.

Is there another way of doing this? I can see the analysisFilePath seems to always be the same as the filePath, except for Razor files. When I open the files in our solution and add TODO comments, the analysis is pretty slow. I can't be sure this is the main issue, but it could be.

We also have another task to not allow a request to be executed for a file that is not in the current solution.

The analysis is 'slow' because 1) it waits for semantic analysis to finish before publishing 2) there's a 500ms delay before triggering the analysis

What's the point of this other task? If the file is not part of the solution it will not be found here

What's the point of this other task?

Security. We should not accept requests for files that are not part of the solution. Also we have to prevent directory traversal attacks

The analysis is 'slow' because 1) it waits for semantic analysis to finish before publishing 2) there's a 500ms delay before triggering the analysis

But traversing all the files for all the projects is also a slow operation

How would it be rejected, based on what? How do you know if a file is part of the solution without looking into the solution?

We will be looking at the solution, of course. And that's the point. That the commands provider would not even be called if the request is for a file that is not part of the solution. My concern is regarding performance. I can update the other task, I guess to remove this check here and think about improving performance.

The only other way to see if the file belongs to the solution is to look at the DTE/IVsHierarchy, and I think that would require us to switch to UI thread, which is worse than the linear search here

The current linear search is searching for x amount of files that have to be analyzed in the y amount of projects.

Anyway the file paths have to be validated at the beginning of the request. I updated the other task and it should be a separate PR

But what's the alternative here? Doing the same but on UI thread?

I am not sure which approach will be taken. It will be defined in the task.
Could be either using IVsHierarchy, either the VsWorkspace with some improvements (e.g. parallelizing the search on different threads).

src/RoslynAnalyzerServer/Analysis/Wrappers/SonarRoslynWorkspaceWrapper.cs

gabriela-trutan-sonarsource · 2025-08-15T08:05:53Z

src/RoslynAnalyzerServer/Analysis/SonarRoslynProjectCompilationProvider.cs

@@ -33,6 +33,8 @@ namespace SonarLint.VisualStudio.RoslynAnalyzerServer.Analysis;
 [method: ImportingConstructor]
 internal class SonarRoslynProjectCompilationProvider(ILogger logger) : ISonarRoslynProjectCompilationProvider
 {
+    private readonly ILogger logger = logger.ForContext("Roslyn Analysis", "Analyzer Exception");


This logger context seems like it's belonging to the class. But the Analyzer Exception context should only be used by OnAnalyzerException. I would split the two contexts: "Roslyn Analysis" for the field logger and move "Analyzer Exception" inside the method

gabriela-trutan-sonarsource · 2025-08-15T08:06:57Z

src/RoslynAnalyzerServer/Analysis/Wrappers/SonarRoslynProjectWrapper.cs


        return analysisFilePath != null;
    }

+    private bool IsCodeBehindRazor(string razorFilePath, string candidateDocumentPath) =>


Maybe the name should be IsGeneratedRazorClass and the comment should be moved as the method doc?

gabriela-trutan-sonarsource · 2025-08-15T08:15:21Z

src/RoslynAnalyzerServer.UnitTests/Analysis/DiagnosticDuplicatesComparerTests.cs

+    [TestMethod]
+    public void GetHashCode_SameObjects_ReturnsSameHashCode()
+    {
+        var diagnostic2 = CreateDiagnostic("rule1", "file1.cs", 1, 1, 1, 10);


This is not why I suggested this change. Is for clean code purposes: if you want to update any of the diagnostic1 field, no adaption would be needed to this class. With the currently hard-coded values, you would have to adapt lots of tests.

My proposal does not change that the assertion happens by value.

gabriela-trutan-sonarsource · 2025-08-15T08:28:09Z

src/RoslynAnalyzerServer/Analysis/SonarDiagnostic.cs

+
+namespace SonarLint.VisualStudio.RoslynAnalyzerServer.Analysis;
+
+public class SonarDiagnostic(


It's not about implementing the model according to the wrong contract. It's about implementing the model differently and then we have more work in the plugin side as well.

For example, the Diagnostic has the primary location information on its level. Instead you created another property called PrimaryLocation, which contains that info. We could have spared us some work if you would have looked at the model and made it consistent.

gabriela-trutan-sonarsource · 2025-08-15T08:34:52Z

src/RoslynAnalyzerServer/Analysis/SonarRoslynSolutionAnalysisCommandProvider.cs

+
+        foreach (var filePath in filePaths)
+        {
+            if (!project.ContainsDocument(filePath, out var analysisFilePath))


How would it be rejected, based on what? How do you know if a file is part of the solution without looking into the solution?

We will be looking at the solution, of course. And that's the point. That the commands provider would not even be called if the request is for a file that is not part of the solution. My concern is regarding performance. I can update the other task, I guess to remove this check here and think about improving performance.

sonarqubecloud · 2025-08-15T10:20:37Z

🤖 Pull Request summary

Refactors SonarCompositeRuleId.GetFullErrorCode method and adds comprehensive Roslyn analysis engine

• Extracted GetFullErrorCode as public static method in SonarCompositeRuleId class with unit tests
• Added complete Roslyn analyzer infrastructure including project compilation, analysis commands, and diagnostic conversion
• Implemented sequential analysis engine with duplicate detection using custom comparer
• Added Roslyn workspace wrappers for testability and abstraction over Visual Studio workspace
• Updated assembly reference baselines showing new Microsoft.CodeAnalysis dependencies

Key areas to focus review on:

The duplicate detection logic in DiagnosticDuplicatesComparer - ensure it properly identifies duplicates across different projects
Error handling and logging in RoslynProjectCompilationProvider.OnAnalyzerException
Performance implications of sequential analysis approach in SequentialRoslynAnalysisEngine
Thread safety considerations in the analysis pipeline components

💬 Please send your feedback

Quality Gate passed

Issues
0 New issues
10 Accepted issues

Measures
0 Security Hotspots
98.2% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[SLVS-2465](https://sonarsource.atlassian.net/browse/SLVS-2465) Part of  [SLVS-2465]: https://sonarsource.atlassian.net/browse/SLVS-2465?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ

georgii-borovinskikh-sonarsource requested a review from gabriela-trutan-sonarsource August 12, 2025 12:32

georgii-borovinskikh-sonarsource marked this pull request as ready for review August 12, 2025 12:32

gabriela-trutan-sonarsource requested changes Aug 13, 2025

View reviewed changes

georgii-borovinskikh-sonarsource requested a review from gabriela-trutan-sonarsource August 14, 2025 13:58

gabriela-trutan-sonarsource approved these changes Aug 15, 2025

View reviewed changes

georgii-borovinskikh-sonarsource added 25 commits August 15, 2025 12:05

SLVS-2465 Add analysis engine to connect with Roslyn APIs

90dfc11

Fix nullability

6c51698

Add compilation wrapper and tests

f42aa88

Add tests for compilation provider

2cd58e3

Add SonarRoslynSolutionAnalysisCommandProviderTests

df0a0d0

add tests & small refactoring

36af208

Cleanup

597d701

Add DiagnosticDuplicatesComparerTests

d45adb3

suppress nullable warnings in tests

d80f882

Add SonarRoslynDiagnosticsConverterTests

a40b77e

fix a few issues

4506974

Add log test

1d424c5

Minor logging improvements

903f853

upd

b33ac0f

Add link to issue streaming ticket

e26d0c6

Add language to diagnostic conversion

cd361df

Add task links

54ab3f2

minor renames

4f2370b

remove unnecessary todos

d3904b5

Move code coverage exclusion

c088874

Fix license headers

836cae2

Minor cleanup

3d11a0e

Fix some of the cr issues

9b073a8

rename classes

1f790d0

Fix tests

83a1936

georgii-borovinskikh-sonarsource added 7 commits August 15, 2025 12:06

Refactor RoslynProjectCompilationProviderTests

329fe4e

Engine test refactoring

e2de4ba

update IsCodeBehindRazor

b3638e6

More refactoring

8114c9c

Rename roslyn issue submodels

67bef50

Minor fix

263c6c2

Update dependency reference files

45f4257

georgii-borovinskikh-sonarsource force-pushed the gb/analysis-engine branch from 2ee49fd to 45f4257 Compare August 15, 2025 10:09

georgii-borovinskikh-sonarsource enabled auto-merge (squash) August 15, 2025 10:13

georgii-borovinskikh-sonarsource merged commit 6a0ce78 into feature/sqvs-roslyn-plugin Aug 15, 2025
3 checks passed

georgii-borovinskikh-sonarsource deleted the gb/analysis-engine branch August 15, 2025 10:20


		namespace SonarLint.VisualStudio.RoslynAnalyzerServer.Analysis;

		public class SonarDiagnostic(

SLVS-2465 Add analysis engine to connect with Roslyn APIs #6368

SLVS-2465 Add analysis engine to connect with Roslyn APIs #6368

Uh oh!

Conversation

georgii-borovinskikh-sonarsource commented Aug 8, 2025 • edited by jira bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gabriela-trutan-sonarsource left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gabriela-trutan-sonarsource Aug 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gabriela-trutan-sonarsource Aug 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sonarqubecloud bot commented Aug 15, 2025

🤖 Pull Request summary

Quality Gate passed

Uh oh!

Uh oh!

georgii-borovinskikh-sonarsource commented Aug 8, 2025 •

edited by jira bot

Loading

gabriela-trutan-sonarsource Aug 15, 2025 •

edited

Loading

gabriela-trutan-sonarsource Aug 15, 2025 •

edited

Loading